Conversation
|
Caution Review failedThe pull request is closed. WalkthroughOpenAI 기반 음성 감정 분석 서비스와 캐싱 모델(weekly_result, frequency_result)을 추가하고, API 엔드포인트 응답 모델을 갱신했으며 오디오 변환·STT 타임아웃·백그라운드 처리를 개선하고 관련 문서를 하나 추가했습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as 클라이언트
participant API as FastAPI 엔드포인트
participant Analysis as analysis_service
participant DB as DB 캐시 (Weekly/Frequency)
participant OpenAI as OpenAI API
Client->>API: GET /voices/analyzing/weekly
API->>Analysis: get_weekly_result(session, username)
Analysis->>DB: 최신 weekly_result 조회 (user)
alt 캐시 존재 (latest_voice_composite_id 동일)
DB-->>Analysis: cached message
Analysis-->>API: WeeklyAnalysisCombinedResponse(합침)
else 캐시 미스
Analysis->>Analysis: _query_weekly_top_emotions() (DB에서 음성 집계)
Analysis->>Analysis: _build_weekly_prompt()
Analysis->>OpenAI: chat completion 요청
OpenAI-->>Analysis: 분석 메시지
Analysis->>DB: WeeklyResult 저장/갱신
Analysis-->>API: WeeklyAnalysisCombinedResponse(합침)
end
API-->>Client: 응답 반환
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (4)
app/services/analysis_service.py (2)
75-75: 사용하지 않는 변수 제거루프 변수
v가 사용되지 않습니다.Based on static analysis
- for v, vc in q: + for _, vc in q: em = (vc.top_emotion or "unknown") if vc else "unknown" cnt[em] += 1
136-145: 예외 메시지를 별도 상수로 정의여러 곳에서 반복되는 문자열 예외 메시지를 별도 상수로 정의하면 유지보수가 용이합니다.
Based on static analysis
파일 상단에 상수 정의:
# 에러 메시지 상수 ERR_USER_NOT_FOUND = "user not found" ERR_INVALID_CARE_USER = "invalid care user or not connected" ERR_CONNECTED_USER_NOT_FOUND = "connected user not found"사용 예:
if not owner: raise ValueError(ERR_USER_NOT_FOUND)app/main.py (2)
365-376: 예외 체이닝 누락예외를 다시 발생시킬 때 원본 예외 컨텍스트가 손실됩니다.
Based on static analysis
except ValueError as e: - raise HTTPException(status_code=400, detail=f"잘못된 요청: {str(e)}") + raise HTTPException(status_code=400, detail=f"잘못된 요청: {str(e)}") from e except Exception as e: - raise HTTPException(status_code=500, detail=f"서버 오류: {str(e)}") + raise HTTPException(status_code=500, detail=f"서버 오류: {str(e)}") from e
365-390: 분석 실패 시 부분 결과 반환 고려OpenAI API 호출 실패 시에도 기존 빈도/주간 데이터는 여전히 유용할 수 있습니다. 현재는 전체 요청이 실패하지만, 메시지 필드를 optional로 만들어 부분 결과를 반환하는 것을 고려해보세요.
예시:
try: message = get_frequency_result(db, username=username, is_care=False) except Exception as e: # OpenAI 실패 시에도 기존 데이터는 제공 logger.warning(f"OpenAI analysis failed: {e}") message = "분석 생성 중 오류가 발생했습니다." voice_service = get_voice_service(db) base = voice_service.get_user_emotion_monthly_frequency(username, month) frequency = base.get("frequency", {}) if base.get("success") else {} return FrequencyAnalysisCombinedResponse(message=message, frequency=frequency)이 경우 DTO에서 message 필드를 Optional로 변경하거나 에러 메시지를 반환하는 방식으로 처리할 수 있습니다.
Also applies to: 544-583
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
GLANCES_USAGE.md(1 hunks)app/dto.py(1 hunks)app/main.py(3 hunks)app/models.py(1 hunks)app/services/analysis_service.py(1 hunks)app/stt_service.py(3 hunks)app/voice_service.py(4 hunks)database_schema.sql(1 hunks)migrations/versions/202511010003_add_weekly_frequency_result.py(1 hunks)migrations/versions/202511030001_merge_heads.py(1 hunks)requirements.txt(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
migrations/versions/202511030001_merge_heads.py (1)
migrations/versions/202511010003_add_weekly_frequency_result.py (2)
upgrade(18-43)downgrade(46-53)
app/main.py (5)
app/dto.py (3)
AnalysisResultResponse(245-248)WeeklyAnalysisCombinedResponse(257-260)FrequencyAnalysisCombinedResponse(263-266)app/database.py (1)
get_db(38-44)app/services/analysis_service.py (2)
get_frequency_result(180-226)get_weekly_result(130-177)app/voice_service.py (3)
get_voice_service(864-866)get_user_emotion_monthly_frequency(774-799)get_user_emotion_weekly_summary(801-861)app/care_service.py (3)
get_emotion_monthly_frequency(12-45)CareService(7-119)get_emotion_weekly_summary(47-119)
app/voice_service.py (4)
app/stt_service.py (1)
transcribe_voice(145-147)app/performance_logger.py (1)
log_step(22-37)app/repositories/job_repo.py (2)
mark_text_done(98-101)try_aggregate(110-160)app/nlp_service.py (1)
analyze_text_sentiment(202-204)
app/services/analysis_service.py (2)
app/models.py (5)
Voice(32-58)VoiceComposite(146-180)User(8-29)WeeklyResult(242-257)FrequencyResult(260-275)app/auth_service.py (1)
get_auth_service(297-299)
🪛 Ruff (0.14.3)
app/main.py
366-366: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
375-375: Do not catch blind exception: Exception
(BLE001)
376-376: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
376-376: Use explicit conversion flag
Replace with conversion flag
(RUF010)
379-379: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
389-389: Do not catch blind exception: Exception
(BLE001)
390-390: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
390-390: Use explicit conversion flag
Replace with conversion flag
(RUF010)
546-546: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
557-557: Do not catch blind exception: Exception
(BLE001)
558-558: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
558-558: Use explicit conversion flag
Replace with conversion flag
(RUF010)
571-571: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
582-582: Do not catch blind exception: Exception
(BLE001)
583-583: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
583-583: Use explicit conversion flag
Replace with conversion flag
(RUF010)
app/voice_service.py
86-86: subprocess call: check for execution of untrusted input
(S603)
99-99: Do not catch blind exception: Exception
(BLE001)
110-110: Do not catch blind exception: Exception
(BLE001)
117-118: try-except-pass detected, consider logging the exception
(S110)
117-117: Do not catch blind exception: Exception
(BLE001)
369-370: try-except-pass detected, consider logging the exception
(S110)
369-369: Do not catch blind exception: Exception
(BLE001)
app/services/analysis_service.py
14-14: Avoid specifying long messages outside the exception class
(TRY003)
75-75: Loop control variable v not used within loop body
Rename unused v to _v
(B007)
136-136: Avoid specifying long messages outside the exception class
(TRY003)
142-142: Avoid specifying long messages outside the exception class
(TRY003)
145-145: Avoid specifying long messages outside the exception class
(TRY003)
186-186: Avoid specifying long messages outside the exception class
(TRY003)
191-191: Avoid specifying long messages outside the exception class
(TRY003)
194-194: Avoid specifying long messages outside the exception class
(TRY003)
🔇 Additional comments (6)
app/models.py (2)
242-257: 캐싱 모델 구조가 적절합니다주간 분석 결과를 캐싱하는 모델 설계가 잘 되어 있습니다:
user_id에 대한 unique constraint로 사용자당 하나의 캐시만 유지latest_voice_composite_id를 통한 캐시 무효화 전략이 명확함- 인덱스 구성이 조회 패턴에 적합함
260-275: FrequencyResult 모델이 WeeklyResult와 일관성 있게 구현되었습니다월간 빈도 분석 캐싱 모델이 주간 분석 모델과 동일한 패턴으로 구현되어 일관성이 좋습니다.
app/services/analysis_service.py (3)
9-15: OpenAI API 키 검증이 적절합니다환경 변수에서 API 키를 로드하고 누락 시 명확한 에러를 발생시키는 로직이 좋습니다.
31-52: 날짜 범위 계산 검증 필요최근 7일 조회 시
start_dt = end_dt - timedelta(days=6)로 계산하고 있는데, 이는 오늘 포함 7일이 맞는지 확인이 필요합니다. 예를 들어 오늘이 11월 3일이면 10월 28일부터 11월 3일까지 7일이 조회됩니다.비즈니스 요구사항이 "지난 7일" (오늘 제외)인 경우 다음과 같이 수정해야 합니다:
start_dt = end_dt - timedelta(days=7) # 그리고 end_dt를 어제로 설정 end_dt = end_dt - timedelta(days=1)
18-28: 제안된 타임아웃 구현 방식이 올바르지 않습니다OpenAI Python 클라이언트에서 타임아웃은
create()메서드의 직접 파라미터로 전달되지 않습니다. 타임아웃은 클라이언트 수준에서 설정하거나with_options()를 통해 설정해야 합니다.올바른 구현은
_get_openai_client()함수에서 타임아웃을 설정하는 것입니다:def _get_openai_client(): """OpenAI 클라이언트 생성 (env에서 키 로드)""" from openai import OpenAI api_key = os.getenv("OPENAI_API_KEY") if not api_key: raise RuntimeError("OPENAI_API_KEY not configured in environment") return OpenAI(api_key=api_key, timeout=30.0)또는 호출 시점에
with_options()을 사용할 수도 있습니다.Likely an incorrect or invalid review comment.
app/main.py (1)
30-31: 새로운 DTO 임포트가 명확합니다분석 결과를 위한 새로운 응답 모델들이 적절히 임포트되었습니다.
| @users_router.get("/voices/analyzing/frequency", response_model=FrequencyAnalysisCombinedResponse) | ||
| async def get_user_emotion_frequency(username: str, month: str, db: Session = Depends(get_db)): | ||
| """사용자 본인의 한달간 감정 빈도수 집계""" | ||
| voice_service = get_voice_service(db) | ||
| result = voice_service.get_user_emotion_monthly_frequency(username, month) | ||
| if not result.get("success"): | ||
| raise HTTPException(status_code=400, detail=result.get("message", "조회 실패")) | ||
| return result | ||
| """사용자 본인의 월간 빈도 종합분석(OpenAI 캐시 + 기존 빈도 결과)""" | ||
| from .services.analysis_service import get_frequency_result | ||
| try: | ||
| message = get_frequency_result(db, username=username, is_care=False) | ||
| voice_service = get_voice_service(db) | ||
| base = voice_service.get_user_emotion_monthly_frequency(username, month) | ||
| frequency = base.get("frequency", {}) if base.get("success") else {} | ||
| return FrequencyAnalysisCombinedResponse(message=message, frequency=frequency) | ||
| except Exception as e: | ||
| raise HTTPException(status_code=400, detail=f"분석 실패: {str(e)}") |
There was a problem hiding this comment.
모든 예외를 400으로 처리하는 것은 부적절합니다
OpenAI API 타임아웃, 데이터베이스 연결 오류 등 서버 측 문제는 500 에러로 처리되어야 합니다. 현재는 모든 예외를 400(클라이언트 오류)으로 반환하여 클라이언트가 재시도 로직을 올바르게 구현하기 어렵습니다.
예외 유형에 따라 적절한 상태 코드를 반환하도록 수정하세요:
- except Exception as e:
- raise HTTPException(status_code=400, detail=f"분석 실패: {str(e)}")
+ except ValueError as e:
+ # 사용자 입력 오류 (user not found 등)
+ raise HTTPException(status_code=400, detail=f"잘못된 요청: {str(e)}")
+ except Exception as e:
+ # 서버 내부 오류 (OpenAI API 오류, DB 오류 등)
+ raise HTTPException(status_code=500, detail=f"서버 오류: {str(e)}")🧰 Tools
🪛 Ruff (0.14.3)
366-366: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
375-375: Do not catch blind exception: Exception
(BLE001)
376-376: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
376-376: Use explicit conversion flag
Replace with conversion flag
(RUF010)
| @users_router.get("/voices/analyzing/weekly", response_model=WeeklyAnalysisCombinedResponse) | ||
| async def get_user_emotion_weekly(username: str, month: str, week: int, db: Session = Depends(get_db)): | ||
| """사용자 본인의 월/주차별 요일별 top 감정 요약""" | ||
| voice_service = get_voice_service(db) | ||
| result = voice_service.get_user_emotion_weekly_summary(username, month, week) | ||
| """사용자 본인의 주간 종합분석(OpenAI 캐시 사용)""" | ||
| from .services.analysis_service import get_weekly_result | ||
| try: | ||
| message = get_weekly_result(db, username=username, is_care=False) | ||
| # 기존 주간 요약도 함께 제공 | ||
| voice_service = get_voice_service(db) | ||
| weekly_result = voice_service.get_user_emotion_weekly_summary(username, month, week) | ||
| weekly = weekly_result.get("weekly", []) if weekly_result.get("success") else [] | ||
| return WeeklyAnalysisCombinedResponse(message=message, weekly=weekly) | ||
| except Exception as e: | ||
| raise HTTPException(status_code=400, detail=f"분석 실패: {str(e)}") |
There was a problem hiding this comment.
get_user_emotion_weekly 엔드포인트에 동일한 예외 처리 이슈
Line 365-376에서 지적한 예외 처리 문제가 이 엔드포인트에도 동일하게 존재합니다.
동일한 수정사항을 적용하세요.
🧰 Tools
🪛 Ruff (0.14.3)
379-379: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
389-389: Do not catch blind exception: Exception
(BLE001)
390-390: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
390-390: Use explicit conversion flag
Replace with conversion flag
(RUF010)
🤖 Prompt for AI Agents
In app/main.py around lines 378-390, the endpoint uses a broad "except Exception
as e" like the earlier block; replace that with targeted error handling by
catching only expected exceptions (or rethrowing unexpected ones), and when
handling errors log the full stacktrace via the application's logger (e.g.,
process_logger.exception) before raising an HTTPException; ensure the
HTTPException returns an appropriate status code (500 for unexpected server
errors or a more specific code for known errors) and a concise detail message
instead of exposing internal stack traces.
| @care_router.get("/users/voices/analyzing/frequency", response_model=FrequencyAnalysisCombinedResponse) | ||
| async def get_emotion_monthly_frequency( | ||
| care_username: str, month: str, db: Session = Depends(get_db) | ||
| ): | ||
| """ | ||
| 보호자 페이지: 연결된 유저의 한달간 감정 빈도수 집계 (CareService 내부 로직 사용) | ||
| """ | ||
| care_service = CareService(db) | ||
| return care_service.get_emotion_monthly_frequency(care_username, month) | ||
| """보호자: 연결 유저의 월간 빈도 종합분석(OpenAI 캐시 + 기존 빈도 결과)""" | ||
| from .services.analysis_service import get_frequency_result | ||
| try: | ||
| message = get_frequency_result(db, username=care_username, is_care=True) | ||
| from .care_service import CareService | ||
| care_service = CareService(db) | ||
| base = care_service.get_emotion_monthly_frequency(care_username, month) | ||
| frequency = base.get("frequency", {}) if base.get("success") else {} | ||
| return FrequencyAnalysisCombinedResponse(message=message, frequency=frequency) | ||
| except Exception as e: | ||
| raise HTTPException(status_code=400, detail=f"분석 실패: {str(e)}") |
There was a problem hiding this comment.
Care 엔드포인트에도 동일한 예외 처리 이슈
예외 유형 구분 및 예외 체이닝 문제가 동일하게 존재합니다.
Lines 365-376에서 제안한 수정사항을 이 엔드포인트에도 적용하세요.
🧰 Tools
🪛 Ruff (0.14.3)
546-546: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
557-557: Do not catch blind exception: Exception
(BLE001)
558-558: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
558-558: Use explicit conversion flag
Replace with conversion flag
(RUF010)
🤖 Prompt for AI Agents
In app/main.py around lines 544-558, replace the broad "except Exception as e"
with targeted exception handling: let existing HTTPException pass through
(except HTTPException: raise), catch expected domain/validation/db errors (e.g.,
ValueError, sqlalchemy.exc.SQLAlchemyError, or your service-specific exceptions)
and convert them to an HTTPException using "raise HTTPException(status_code=400,
detail=f'분석 실패: {e}') from e" to preserve exception chaining; for any other
unexpected exceptions you can log them and raise a generic HTTPException with
chaining as well. Ensure you import any exception classes you reference (e.g.,
SQLAlchemyError) and use "from e" when re-raising to keep the original
traceback.
| @care_router.get("/users/voices/analyzing/weekly", response_model=WeeklyAnalysisCombinedResponse) | ||
| async def get_emotion_weekly_summary( | ||
| care_username: str, | ||
| month: str, | ||
| week: int, | ||
| db: Session = Depends(get_db) | ||
| ): | ||
| """보호자페이지 - 연결유저 월/주차별 요일 top 감정 통계""" | ||
| care_service = CareService(db) | ||
| return care_service.get_emotion_weekly_summary(care_username, month, week) | ||
| """보호자: 연결 유저의 주간 종합분석(OpenAI 캐시 사용)""" | ||
| from .services.analysis_service import get_weekly_result | ||
| try: | ||
| message = get_weekly_result(db, username=care_username, is_care=True) | ||
| # 기존 주간 요약도 함께 제공 | ||
| care_service = CareService(db) | ||
| weekly_result = care_service.get_emotion_weekly_summary(care_username, month, week) | ||
| weekly = weekly_result.get("weekly", []) if weekly_result.get("success") else [] | ||
| return WeeklyAnalysisCombinedResponse(message=message, weekly=weekly) | ||
| except Exception as e: | ||
| raise HTTPException(status_code=400, detail=f"분석 실패: {str(e)}") |
There was a problem hiding this comment.
Care weekly 엔드포인트에도 동일한 예외 처리 이슈
모든 분석 엔드포인트에서 일관되게 예외 처리를 개선해야 합니다.
Lines 365-376에서 제안한 예외 처리 패턴을 이 엔드포인트에도 적용하세요.
🧰 Tools
🪛 Ruff (0.14.3)
571-571: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
582-582: Do not catch blind exception: Exception
(BLE001)
583-583: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
583-583: Use explicit conversion flag
Replace with conversion flag
(RUF010)
🤖 Prompt for AI Agents
In app/main.py around lines 566 to 583, the endpoint uses a broad except that
returns a 400 with only the exception string; apply the same improved
exception-handling pattern used at lines 365-376: replace the broad except with
targeted handling (catch expected service/validation errors and raise
HTTPException with appropriate status and user-facing detail), and for any
unexpected Exception call logger.exception(...) to record the full stack trace
and then raise HTTPException(status_code=500, detail="서버 오류가 발생했습니다") so
internal errors are logged but not leaked to clients.
| def get_weekly_result(session: Session, username: str, is_care: bool = False) -> str: | ||
| """주간 종합분석 결과 메시지 생성""" | ||
| from ..auth_service import get_auth_service | ||
| auth = get_auth_service(session) | ||
| owner = auth.get_user_by_username(username) | ||
| if not owner: | ||
| raise ValueError("user not found") | ||
|
|
||
| # care인 경우 연결된 유저로 전환 | ||
| target_user = owner | ||
| if is_care: | ||
| if owner.role != 'CARE' or not owner.connecting_user_code: | ||
| raise ValueError("invalid care user or not connected") | ||
| target_user = auth.get_user_by_username(owner.connecting_user_code) | ||
| if not target_user: | ||
| raise ValueError("connected user not found") | ||
|
|
||
| # 최신 voice_composite_id 조회 | ||
| latest_vc = ( | ||
| session.query(VoiceComposite.voice_composite_id) | ||
| .join(Voice, Voice.voice_id == VoiceComposite.voice_id) | ||
| .filter(Voice.user_id == target_user.user_id) | ||
| .order_by(VoiceComposite.created_at.desc()) | ||
| .first() | ||
| ) | ||
| latest_vc_id = latest_vc[0] if latest_vc else None | ||
|
|
||
| # 캐시 조회 | ||
| cache = session.query(WeeklyResult).filter(WeeklyResult.user_id == target_user.user_id).first() | ||
| if cache and cache.latest_voice_composite_id == latest_vc_id: | ||
| return cache.message | ||
|
|
||
| # 생성 후 캐시 저장/갱신 | ||
| by_day = _query_weekly_top_emotions(session, target_user.user_id) | ||
| messages = _build_weekly_prompt(target_user.name, by_day) | ||
| msg = _call_openai(messages) | ||
|
|
||
| if cache: | ||
| cache.message = msg | ||
| cache.latest_voice_composite_id = latest_vc_id | ||
| else: | ||
| session.add(WeeklyResult( | ||
| user_id=target_user.user_id, | ||
| latest_voice_composite_id=latest_vc_id, | ||
| message=msg, | ||
| )) | ||
| session.commit() | ||
| return msg |
There was a problem hiding this comment.
캐시 무효화 로직에 잠재적 경쟁 조건 존재
여러 요청이 동시에 들어올 경우 같은 사용자에 대해 중복으로 OpenAI API를 호출하고 캐시를 업데이트할 수 있습니다. 캐시 확인(line 158-160)과 OpenAI 호출(line 165) 사이에 다른 요청이 캐시를 업데이트할 수 있습니다.
다음 해결 방안을 고려하세요:
- 데이터베이스 레벨 락 사용 (SELECT FOR UPDATE)
- Redis 등 분산 락 사용
- 중복 호출을 허용하되 마지막 업데이트만 반영 (현재 방식 유지하되 문서화)
옵션 1 (SELECT FOR UPDATE) 예시:
# 캐시 조회 시 락 획득
cache = session.query(WeeklyResult).filter(
WeeklyResult.user_id == target_user.user_id
).with_for_update().first()참고: 이 방식은 트랜잭션 격리 수준과 데드락 가능성을 고려해야 합니다.
🧰 Tools
🪛 Ruff (0.14.3)
136-136: Avoid specifying long messages outside the exception class
(TRY003)
142-142: Avoid specifying long messages outside the exception class
(TRY003)
145-145: Avoid specifying long messages outside the exception class
(TRY003)
🤖 Prompt for AI Agents
In app/services/analysis_service.py around lines 130-177, the cache-validation
-> OpenAI call has a race where concurrent requests can both call OpenAI and
overwrite the cache; fix by acquiring a lock before deciding to call OpenAI:
begin a DB transaction, re-query the WeeklyResult for target_user.user_id using
SELECT ... FOR UPDATE (SQLAlchemy: with_for_update()) to lock the row (or use a
distributed lock like Redis if preferred), re-check latest_voice_composite_id
against latest_vc_id inside the lock/transaction, only call OpenAI if the cached
message is still stale, then update or insert the WeeklyResult and commit the
transaction; be mindful of transaction boundaries/timeout and potential
deadlocks and choose Redis locks if you need cross-process non-blocking
behavior.
트랜잭션 롤백 처리 누락
OpenAI API 호출(line 165) 실패 시 이미 시작된 트랜잭션이 롤백되지 않고 열린 상태로 남을 수 있습니다. session.commit() (line 176) 전에 예외가 발생하면 세션이 일관되지 않은 상태가 됩니다.
try-except-finally 블록으로 트랜잭션 관리를 명확히 하세요:
def get_weekly_result(session: Session, username: str, is_care: bool = False) -> str:
"""주간 종합분석 결과 메시지 생성"""
from ..auth_service import get_auth_service
auth = get_auth_service(session)
owner = auth.get_user_by_username(username)
if not owner:
raise ValueError("user not found")
# care인 경우 연결된 유저로 전환
target_user = owner
if is_care:
if owner.role != 'CARE' or not owner.connecting_user_code:
raise ValueError("invalid care user or not connected")
target_user = auth.get_user_by_username(owner.connecting_user_code)
if not target_user:
raise ValueError("connected user not found")
# 최신 voice_composite_id 조회
latest_vc = (
session.query(VoiceComposite.voice_composite_id)
.join(Voice, Voice.voice_id == VoiceComposite.voice_id)
.filter(Voice.user_id == target_user.user_id)
.order_by(VoiceComposite.created_at.desc())
.first()
)
latest_vc_id = latest_vc[0] if latest_vc else None
# 캐시 조회
cache = session.query(WeeklyResult).filter(WeeklyResult.user_id == target_user.user_id).first()
if cache and cache.latest_voice_composite_id == latest_vc_id:
return cache.message
+ try:
# 생성 후 캐시 저장/갱신
by_day = _query_weekly_top_emotions(session, target_user.user_id)
messages = _build_weekly_prompt(target_user.name, by_day)
msg = _call_openai(messages)
if cache:
cache.message = msg
cache.latest_voice_composite_id = latest_vc_id
else:
session.add(WeeklyResult(
user_id=target_user.user_id,
latest_voice_composite_id=latest_vc_id,
message=msg,
))
session.commit()
return msg
+ except Exception:
+ session.rollback()
+ raise🧰 Tools
🪛 Ruff (0.14.3)
136-136: Avoid specifying long messages outside the exception class
(TRY003)
142-142: Avoid specifying long messages outside the exception class
(TRY003)
145-145: Avoid specifying long messages outside the exception class
(TRY003)
🤖 Prompt for AI Agents
In app/services/analysis_service.py around lines 130-177, the OpenAI call and
subsequent DB write run inside an open transaction without rollback on failure;
wrap the generation and DB mutation logic (from _call_openai through
session.commit()) in a try/except that calls session.rollback() if any exception
occurs, optionally log or attach error context, then re-raise the exception so
callers handle it, and ensure session.commit() remains in the successful path
(or use try/except/finally to guarantee rollback on error and commit only on
success).
| def get_frequency_result(session: Session, username: str, is_care: bool = False) -> str: | ||
| """월간 빈도 종합분석 결과 메시지 생성""" | ||
| from ..auth_service import get_auth_service | ||
| auth = get_auth_service(session) | ||
| owner = auth.get_user_by_username(username) | ||
| if not owner: | ||
| raise ValueError("user not found") | ||
|
|
||
| target_user = owner | ||
| if is_care: | ||
| if owner.role != 'CARE' or not owner.connecting_user_code: | ||
| raise ValueError("invalid care user or not connected") | ||
| target_user = auth.get_user_by_username(owner.connecting_user_code) | ||
| if not target_user: | ||
| raise ValueError("connected user not found") | ||
|
|
||
| # 최신 voice_composite_id 조회 | ||
| latest_vc = ( | ||
| session.query(VoiceComposite.voice_composite_id) | ||
| .join(Voice, Voice.voice_id == VoiceComposite.voice_id) | ||
| .filter(Voice.user_id == target_user.user_id) | ||
| .order_by(VoiceComposite.created_at.desc()) | ||
| .first() | ||
| ) | ||
| latest_vc_id = latest_vc[0] if latest_vc else None | ||
|
|
||
| # 캐시 조회 | ||
| cache = session.query(FrequencyResult).filter(FrequencyResult.user_id == target_user.user_id).first() | ||
| if cache and cache.latest_voice_composite_id == latest_vc_id: | ||
| return cache.message | ||
|
|
||
| # 생성 후 캐시 저장/갱신 | ||
| counts = _query_month_emotion_counts(session, target_user.user_id) | ||
| messages = _build_frequency_prompt(target_user.name, counts) | ||
| msg = _call_openai(messages) | ||
|
|
||
| if cache: | ||
| cache.message = msg | ||
| cache.latest_voice_composite_id = latest_vc_id | ||
| else: | ||
| session.add(FrequencyResult( | ||
| user_id=target_user.user_id, | ||
| latest_voice_composite_id=latest_vc_id, | ||
| message=msg, | ||
| )) | ||
| session.commit() | ||
| return msg |
There was a problem hiding this comment.
get_frequency_result 함수에 동일한 이슈 존재
get_weekly_result에서 지적한 경쟁 조건 및 트랜잭션 롤백 문제가 이 함수에도 동일하게 존재합니다.
get_weekly_result에 적용한 수정사항을 이 함수에도 동일하게 적용하세요.
🧰 Tools
🪛 Ruff (0.14.3)
186-186: Avoid specifying long messages outside the exception class
(TRY003)
191-191: Avoid specifying long messages outside the exception class
(TRY003)
194-194: Avoid specifying long messages outside the exception class
(TRY003)
| sa.Column('created_at', sa.DateTime(), server_default=sa.text('CURRENT_TIMESTAMP'), nullable=False), | ||
| sa.Column('updated_at', sa.DateTime(), server_default=sa.text('CURRENT_TIMESTAMP'), nullable=False), | ||
| sa.UniqueConstraint('user_id', name='uq_weekly_user'), | ||
| ) | ||
| op.create_index('idx_weekly_user', 'weekly_result', ['user_id']) | ||
| op.create_index('idx_weekly_latest_vc', 'weekly_result', ['latest_voice_composite_id']) | ||
|
|
||
| op.create_table( | ||
| 'frequency_result', | ||
| sa.Column('frequency_result_id', sa.BigInteger(), primary_key=True, autoincrement=True), | ||
| sa.Column('user_id', sa.BigInteger(), sa.ForeignKey('user.user_id', ondelete='CASCADE'), nullable=False), | ||
| sa.Column('latest_voice_composite_id', sa.BigInteger(), sa.ForeignKey('voice_composite.voice_composite_id', ondelete='CASCADE'), nullable=True), | ||
| sa.Column('message', sa.Text(), nullable=False), | ||
| sa.Column('created_at', sa.DateTime(), server_default=sa.text('CURRENT_TIMESTAMP'), nullable=False), | ||
| sa.Column('updated_at', sa.DateTime(), server_default=sa.text('CURRENT_TIMESTAMP'), nullable=False), | ||
| sa.UniqueConstraint('user_id', name='uq_freq_user'), |
There was a problem hiding this comment.
updated_at 자동 갱신이 빠져 있습니다
이 테이블은 캐시된 분석 결과를 계속 갱신할 예정인데, 현재 migration에서는 updated_at 컬럼에 server_onupdate가 설정되지 않아 UPDATE 시각이 자동으로 변하지 않습니다. 이후 캐시 무효화나 정합성 판단에서 시간이 갱신되지 않아 오래된 데이터로 오인될 가능성이 큽니다. database_schema.sql에도 ON UPDATE CURRENT_TIMESTAMP가 명시된 만큼 migration에서도 동일하게 맞춰주세요. 아래처럼 server_onupdate=sa.text('CURRENT_TIMESTAMP')를 추가하면 해결됩니다.
- sa.Column('updated_at', sa.DateTime(), server_default=sa.text('CURRENT_TIMESTAMP'), nullable=False),
+ sa.Column(
+ 'updated_at',
+ sa.DateTime(),
+ server_default=sa.text('CURRENT_TIMESTAMP'),
+ server_onupdate=sa.text('CURRENT_TIMESTAMP'),
+ nullable=False,
+ ),두 번째 테이블(frequency_result)도 동일하게 수정해 주세요.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In migrations/versions/202511010003_add_weekly_frequency_result.py around lines
25 to 40, the updated_at columns for the weekly_result and frequency_result
tables lack automatic update behavior; add
server_onupdate=sa.text('CURRENT_TIMESTAMP') to each updated_at Column (i.e.,
change server_default=sa.text('CURRENT_TIMESTAMP') to also include
server_onupdate=sa.text('CURRENT_TIMESTAMP')) so UPDATE operations refresh the
timestamp and match database_schema.sql's ON UPDATE CURRENT_TIMESTAMP.
1 participant
🔎 Description
🔗 Related Issue
🏷️ What type of PR is this?
📋 Changes Made
🧪 Testing
📸 Screenshots (if applicable)
📝 Additional Notes
Summary by CodeRabbit
새로운 기능
개선사항
데이터베이스
문서